Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Eric proj4 #5

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Eric proj4 #5

wants to merge 4 commits into from

Conversation

aneeshj4
Copy link
Contributor

Fix

@Override
public void onClick(View v){
switch (v.getId()) {
case R.id.createEventButton:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code in each case in this switch statement could be modularized into functions to make it easier to read. Like you could have createEventButtonClicked()

break;
default:
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I usually see onCreate at the top of the class. Idk how other people feel about it but that's what I've seen the most.

protected void onActivityResult(int requestCode, int resultCode, Intent data) {
super.onActivityResult(requestCode, resultCode, data);
//Detects request codes
if ((requestCode == 3 || requestCode == 1) && resultCode == Activity.RESULT_OK) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do 3 and 1 represent? Define them as public static final ints at the top so we know what request each corresponds to. In the 61B style guide, these are referred to as "magic numbers"

@Override
public void onClick(View v){ // Handles Cases for clicking Interested & Num RSVP Buttons
switch (v.getId()) {
case R.id.numRSVPButton:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code in each case in this switch statement could be modularized into functions to make it easier to read. Like you could have numRSVPButtonClicked(), interestedButtonClicked(), etc

mSocials.clear();
for (DataSnapshot socialSnapShot: dataSnapshot.getChildren()) {
GenericTypeIndicator<ArrayList<String>> t = new GenericTypeIndicator<ArrayList<String>>() {};
Social social = new Social(socialSnapShot.child(getString(R.string.eventName)).getValue(String.class),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can give Firebase enter classes, like snapshot.getValue(Social.class) and it'll populate the fields based on the variable names. It is useful in cases like this.

}
}

protected void onProgressUpdate(Void... progress) {}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this function isn't used then don't include it.

new DownloadFilesTaskSocial().execute(uri.toString());

}
}).addOnFailureListener(new OnFailureListener() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a need for this failure listener? If it isn't used?

public void onClick(View view) {
Intent intent = new Intent(context, DetailActivity.class);
String socialId = s.eventImage.substring(0, s.eventImage.length() - 4);
intent.putExtra("socialID", socialId);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can define the strings used as the extra keys as final variables at the top of the class to prevent typos (because you'll be using these same strings again when extracting the extras)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants